feat(kiro): wire up new file-based session format (~May 2026)#268
feat(kiro): wire up new file-based session format (~May 2026)#268Sean10 wants to merge 1 commit into
Conversation
Kiro CLI moved to per-session files under ~/.kiro/sessions/cli/:
<uuid>.json — metadata (session_id, cwd, title, created_at, updated_at)
<uuid>.jsonl — events: Prompt / AssistantMessage / ToolResults
- Add KIRO_SESSIONS_DIR + scanKiroCliSessions() and loadKiroCliDetail()
- Index the .jsonl files in _buildSessionFileIndex so they resolve to
{ format: 'kiro-cli' }, wiring detail/preview/search/replay/export
end-to-end (previously listed but "Session file not found" on open)
- Validate the sessionId as a strict UUID in loadKiroCliDetail before
path.join to close a path-traversal vector on untrusted input
- Add fixture-based tests covering scan, detail, path-traversal
rejection, index resolution, and end-to-end wiring
Old SQLite-based Kiro sessions remain fully supported alongside.
vakovalskii
left a comment
vakovalskii
left a comment
There was a problem hiding this comment.
Thanks for this — the file-based Kiro reader works and merges cleanly (all tests green, incl. the new ones). One blocker before merge, from a security pass:
Symlink traversal when reading session files (HIGH). The <uuid>.jsonl filenames are correctly UUID-validated (so ../ traversal is blocked), but fs.readFileSync(path.join(KIRO_SESSIONS_DIR, f)) still follows symlinks. A symlink named <valid-uuid>.jsonl inside ~/.kiro/sessions/cli/ pointing to e.g. ~/.ssh/id_rsa would be read and surfaced through the dashboard (detail / export / search / replay). This matters because codbash can be bound to a LAN address, and it's inconsistent with how the rest of the codebase already defends against this — see the Claude reader (src/data.js, ~line 679: if (entry.isSymbolicLink()) continue;) and isSafeLaunchPath in projects.js.
Two read sites are affected:
- the session-list loader (
readFileSyncof the meta/.jsonl) - the detail loader (
readFileSync(jsonlPath, ...))
Fix direction: before each read, reject symlinks / enforce containment, e.g.
const st = fs.lstatSync(p);
if (st.isSymbolicLink() || !st.isFile()) continue; // or: return null in the detail pathor fs.realpathSync(p) and verify it still starts with KIRO_SESSIONS_DIR + path.sep. Prefer lstat + isSymbolicLink to match the existing pattern.
No command-injection or prototype-pollution issues found in the new code — just this. Happy to merge once the symlink guard is in. 🙏
2 participants
Follow-up to #230. Part 1 (maxBuffer +
ToolUse) already landed inf2b77f9; this PR is Part 2 reworked around @vakovalskii's review feedback.What & why
Kiro CLI moved to per-session files under
~/.kiro/sessions/cli/:<uuid>.json— metadata (session_id,cwd,title,created_at,updated_at)<uuid>.jsonl— events, one per line:Prompt→ user,AssistantMessage→ assistant,ToolResults→ skippedThe previous version of Part 2 listed these sessions but wasn't wired end-to-end — detail/preview/search/replay/export all resolve through
findSessionFile()/_buildSessionFileIndex(), which didn't know aboutkiro-cli, so opening a session returned "Session file not found" and the newfound.format === 'kiro-cli'branches were never reached.Changes (addressing review points)
_buildSessionFileIndex()now scans~/.kiro/sessions/cli/*.jsonland registers them as{ format: 'kiro-cli', sessionId }, sofindSessionFile()resolves them and every downstream consumer (detail, preview, search, replay, export) works end-to-end.loadKiroCliDetail()validatessessionIdas a strict UUID beforepath.join, since it takes untrusted request input once wired. The scanner uses the same strict UUID pattern.test/kiro-cli-session.test.jscovers scan, detail parsing, path-traversal rejection, index resolution, and full end-to-end wiring (detail/preview/replay/export).scanKiroCliSessions()+loadKiroCliDetail()carry the parsing; old SQLite-based Kiro sessions remain fully supported alongside the new format.Testing
node --test test/*.test.js→ 207 pass, 2 skipped (win32-only), 0 fail